[codex] fix runtime guard stale log filtering - #282
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce9626be28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| matches = [ | ||
| service_since | ||
| for service, service_since in service_since_by_name.items() | ||
| if any(alias and alias in job_name for alias in _service_job_aliases(service)) | ||
| ] |
There was a problem hiding this comment.
When two monitored services share a prefix, this substring check can associate a scheduler log with the wrong service window. For example, with foo-service and foo-bar-service, the foo-bar-scheduler job contains the foo alias, so max(matches) may pick foo-service's later ready-revision time and drop a real foo-bar scheduler failure as stale. Match against the expected scheduler job names or otherwise choose the service-specific alias unambiguously before filtering failures.
Useful? React with 👍 / 👎.
| text = str(value).strip().lower() | ||
| if not text: | ||
| return default | ||
| return text in {"1", "true", "yes", "y", "on"} |
There was a problem hiding this comment.
When a target has a non-empty RUNTIME_TARGET_ENABLED value outside the accepted boolean spellings, this returns False, so a typo such as treu silently removes that Cloud Run service from runtime-guard monitoring. Other configuration paths treat this flag as enabled by default and fail invalid values (scripts/build_cloud_run_env_sync_plan.py:378-386), so this should reject or ignore unknown values instead of suppressing alerts.
Useful? React with 👍 / 👎.
| for key in ("runtime_target_enabled", "RUNTIME_TARGET_ENABLED"): | ||
| if key in target: | ||
| return _coerce_bool(target.get(key), True) | ||
| if key in runtime_target: | ||
| return _coerce_bool(runtime_target.get(key), True) |
There was a problem hiding this comment.
When the targets JSON uses the same shape as env sync, RUNTIME_TARGET_ENABLED may come from target.env, defaults, or defaults.env, but this helper only checks the top-level target and nested runtime target. In that case env sync deploys the service with RUNTIME_TARGET_ENABLED=false while the runtime guard still loads and alerts on it, so disabled targets are not actually ignored for that supported configuration shape.
Useful? React with 👍 / 👎.
1 participant
Summary
Test Plan